Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rpc: Don't ignore -maxtxfee when wallet is disabled #15357

Merged
merged 1 commit into from Feb 8, 2019
Merged

rpc: Don't ignore -maxtxfee when wallet is disabled #15357

merged 1 commit into from Feb 8, 2019
+16 −15

Conversation

JBaczuk
Copy link
Contributor

@JBaczuk JBaczuk commented Feb 6, 2019

Resolves #15355

ryanofsky
ryanofsky reviewed Feb 6, 2019
View reviewed changes
src/init.cpp Outdated Show resolved Hide resolved
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 6, 2019 •
edited

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15340 (gui: Introduce bilingual GUI error messages by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@JBaczuk
Copy link
Contributor Author

JBaczuk commented Feb 7, 2019

Ok I've tested and it does abort if invalid, and issues a warning if fee is too high.

@Empact
Copy link
Member

Empact commented Feb 7, 2019

Maybe move it up to around line 1099, to sit alongside the other fee-related options handling?

@maflcko maflcko changed the title Move maxTxFee initialization to init.cpp rpc: Don't ignore -maxtxfee when wallet is disabled Feb 7, 2019
@JBaczuk
Copy link
Contributor Author

JBaczuk commented Feb 8, 2019 •
edited

Maybe move it up to around line 1099, to sit alongside the other fee-related options handling?

done

@bitcoin bitcoin deleted a comment from weerayut3 Feb 8, 2019
@maflcko
Copy link
Member

maflcko commented Feb 8, 2019

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits. There is no need to keep the git history around in this pull request.

@jnewbery
Copy link
Contributor

jnewbery commented Feb 8, 2019

utACK dfbf117 because it at least makes behaviour consistent. I still don't think that -maxtxfee should be shared between the node and wallet, so this doesn't fully address my concerns in #15355. Reasoning here: #15355 (comment).

@maflcko
Copy link
Member

maflcko commented Feb 8, 2019

ACK dfbf117.

Checked that it is no longer ignored when the wallet is not compiled or not enabled:

$ ./configure --disable-wallet && make
$ ./src/qt/bitcoin-qt -maxtxfee=foobar
Error: Invalid amount for -maxtxfee=<amount>: 'foobar'

$ ./configure && make
$ ./src/qt/bitcoin-qt -maxtxfee=foobar -disablewallet
Error: Invalid amount for -maxtxfee=<amount>: 'foobar'

screenshot from 2019-02-08 11-23-11

@maflcko maflcko added this to the 0.18.0 milestone Feb 8, 2019
ryanofsky
ryanofsky approved these changes Feb 8, 2019
View reviewed changes
Copy link
Contributor

@ryanofsky ryanofsky left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK dfbf117, confirmed move only.

@maflcko maflcko merged commit dfbf117 into bitcoin:master Feb 8, 2019
maflcko pushed a commit that referenced this pull request Feb 8, 2019
Merge #15357: rpc: Don't ignore -maxtxfee when wallet is disabled
2945492 
dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves #15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
@jnewbery jnewbery mentioned this pull request Feb 8, 2019
Closed
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 27, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
139a985 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 28, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
56a4ffa 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 29, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
5e9a6ba 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 1, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
504f40e 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 5, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
e476094 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 7, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
4efa0a6 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 7, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
3a9a1cb 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 9, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
c4b8697 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 10, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
2d5cdd4 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Sep 13, 2021
@Munkybooty
Merge bitcoin#15357: rpc: Don't ignore -maxtxfee when wallet is dis…
ae4bea6 
…abled

dfbf117 Move maxTxFee initialization to init.cpp (Jordan Baczuk)

Pull request description:

  Resolves bitcoin#15355

Tree-SHA512: 6eafacc6a3b0589fb645b0080fd3c01598566df1bd7ee7929284853866a23493960fbd4d6f9c3417e192f8a21706d9f593197734f6189e046e4747991305a0b8
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@maflcko maflcko maflcko left review comments

@ryanofsky ryanofsky ryanofsky approved these changes

Assignees
No one assigned
Labels
RPC/REST/ZMQ
Projects
None yet
Milestone
0.18.0
Development

Successfully merging this pull request may close these issues.

-maxtxfee should not be used by both node and wallet
7 participants
@JBaczuk @DrahtBot @Empact @maflcko @jnewbery @ryanofsky @fanquake